-
Notifications
You must be signed in to change notification settings - Fork 92
Use AdoptOpenJDK JDK 8 and 11 #334
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
f20da4a
to
6842ed2
Compare
install: | ||
- sdk install java $(sdk list java | grep -o "$ADOPTOPENJDK\.[0-9\.]*hs-adpt" | head -1) | ||
- unset _JAVA_OPTIONS | ||
- unset JAVA_HOME |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you explain this line? Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
JAVA_HOME is set by Travis CI's base image or language: scala
to be whatevet the JDK, like JDK 11 for xenial. since I'm installing a new JDK I don't want the java on PATH and JAVA_HOME to be not inconsistent. This normally only matters when there's forking or Java only project. Alternatively I could try to do which java
and set JAVA_HOME to wherever SDKMAN installs AdoptOpenJDK JDK.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, so sdk install
adds the installed version to the path, but doesn't set JAVA_HOME
. I'm fine with unsetting if we don't need it. For the record, jabba use
sets both the PATH
and JAVA_HOME
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, can be squashed.
529d32c
to
d90955e
Compare
Squashed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, looking at the validation of this PR, I see that travis is not actually enabled on this repo. It seems to use circle CI...
@lrytz lol. good catch. I saw those too but somehow I didn't put 2 and 2 together. I guess I'd use adoptopenjdk/openjdk8 or eed3si9n/sbt:jdk8-alpine Docker image. |
@ashawley I am not familiar with CircleCI, but I am guessing that creating a custom image of a drop-in replacement of the current image using AdoptOpenJDK is going to be some work, so what do you think about switching back to Travis CI for now? |
I haven't touched TravisCI. It should still be enabled for the repo. Is there an issue with Travis? Is there a syntax error in your yaml file? The travis UI changed recently and you have to find those errors from the requests page from under the hamburger menu. |
Interesting. We noticed it's no longer running, but maybe you're right about it potentially being an YAML error etc. The last known build on Travis CI was from this PR yesterday - https://travis-ci.org/scala/scala-xml/builds/556385195/config |
d90955e
to
2998e29
Compare
@ashawley you were totally right about YAML syntax. |
Looks like it ran on Travis CI fine (in addition to Circle CI), so I am going to self-merge. |
Yeah, I've ever been bit by both the YAML syntax problem, and the hidden error problem in Travis. |
👍 funny episode |
No description provided.